Skip to content

Add diagnostic logging to Delete action in CleanAcrImagesCommand#2057

Open
Copilot wants to merge 3 commits intomainfrom
copilot/update-cleanup-command-repo-date
Open

Add diagnostic logging to Delete action in CleanAcrImagesCommand#2057
Copilot wants to merge 3 commits intomainfrom
copilot/update-cleanup-command-repo-date

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 30, 2026 •
edited
Loading

Duplicate EOL annotation writes continuously refresh LastUpdatedOn on staging repos, so the Delete action's age check never passes and repos accumulate indefinitely (1,840+ staging repos).

To safely evaluate the correct fix, this PR adds structured diagnostic logging to the Delete case in CleanAcrImagesCommand.ProcessRepoAsync. The log output includes:

  • Repository name
  • CreatedOn date
  • LastUpdatedOn date
  • Manifest count
  • Whether the repo is being deleted
  • Reason why/why not

This will allow analysis of pipeline log output to determine whether switching from LastUpdatedOn to CreatedOn is the correct approach before making that behavioral change.

Original prompt

Based on #2056, update the cleanup command to use the repo's created date instead of the last updated date.

@lbussell
Use repo CreatedOn date instead of LastUpdatedOn for Delete action in…
8e51b18 
… cleanup command

Fixes #2056: EOL annotation writes keep updating LastUpdatedOn on staging
repos, preventing the 15-day age check from ever passing. Using CreatedOn
ensures repos are deleted based on when they were originally created.

Agent-Logs-Url: https://github.com/dotnet/docker-tools/sessions/0a39b8b7-2866-4759-ab63-7d2d91f58b4a

Co-authored-by: lbussell <36081148+lbussell@users.noreply.github.com>
Copilot AI changed the title [WIP] Update cleanup command to use repo's created date Use repo CreatedOn instead of LastUpdatedOn for Delete action in cleanup command Mar 30, 2026
Copilot AI requested a review from lbussell March 30, 2026 23:12
@lbussell
Copy link
Copy Markdown
Member

lbussell commented Mar 30, 2026

@copilot on second thought, this change seems risky right out of the gate. We might end up deleting more repos than we intended. Do this instead:

Instead, add logging output underneath the case CleanAcrImagesAction.Delete: switch case. The log event should include: created date, last updated date, number of manifests, whether or not it's being deleted, and why/why not. Follow best practices for structured logging using Microsoft.Extensions.Logging.

That's it for this PR. Then, we'll be able to analyze the log output and determine if the repo CreatedOn date is the correct decision.

@lbussell
Revert CreatedOn change; add diagnostic logging to Delete action
8488aa4 
Reverts the CreatedOn change and instead adds structured logging to the
Delete case that includes: created date, last updated date, manifest
count, whether the repo is being deleted, and the reason why/why not.
This will enable log analysis to determine the correct date field to use.

Agent-Logs-Url: https://github.com/dotnet/docker-tools/sessions/9a845109-70e8-4747-a5d2-4f3ded9726e0

Co-authored-by: lbussell <36081148+lbussell@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 30, 2026

@copilot on second thought, this change seems risky right out of the gate. We might end up deleting more repos than we intended. Do this instead:

Instead, add logging output underneath the `case Cl...

Reverted the CreatedOn change and added structured logging instead in 8488aa4. The Delete case now logs: RepositoryName, CreatedOn, LastUpdatedOn, ManifestCount, Deleting (bool), and Reason (explains why/why not). The existing LastUpdatedOn behavior is preserved.

Copilot AI changed the title Use repo CreatedOn instead of LastUpdatedOn for Delete action in cleanup command Add diagnostic logging to Delete action in CleanAcrImagesCommand Mar 30, 2026
@lbussell lbussell marked this pull request as ready for review March 30, 2026 23:34
@lbussell lbussell requested a review from a team as a code owner March 30, 2026 23:34
@lbussell lbussell enabled auto-merge (squash) March 30, 2026 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NikolaMilosavljevic NikolaMilosavljevic Awaiting requested review from NikolaMilosavljevic

1 more reviewer

@lbussell lbussell lbussell approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@lbussell lbussell

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lbussell